Verdict: NIT (one band-aid risk worth flagging — see #1)

Head: 7498be8. Base: chore/studio-remove-console-logs (Graphite stack — verify final-state at merge time). The intent of every chunk is sound and the comments explaining the GSAP no-op-at-creation-position quirk are exemplary. Three observations, one of them a Pattern #2 band-aid the author may want to address before this lands.

1. bindRootTimelineIfAvailable no longer restores state.currentTime on rebind (potential regression)

packages/core/src/runtime/init.ts ~L1029–1052. Pre-PR the boundDuration > 0 branch did:

state.capturedTimeline.pause();
const seekTime = Math.max(0, state.currentTime || 0);
state.capturedTimeline.totalTime(seekTime, false);

Post-PR it does:

state.capturedTimeline.progress(0.0001, true);
state.capturedTimeline.progress(0, false);
state.capturedTimeline.pause();

The progress-cycle is the right kick — it correctly forces GSAP to render tl.set() at position 0 instead of no-op'ing because the playhead never moved off the creation point. But it unconditionally lands at progress(0), whereas the previous code restored state.currentTime. bindRootTimelineIfAvailable is called from the rAF transport loop every 60 frames (~1s) at L2069/L2272, plus other late-bind sites — if a child composition timeline registers after the user has scrubbed (or after a soft reload restores a non-zero currentTime), the rebind will now snap the playhead back to 0. The sibling rebindTimelineFromResolution at L1336 still does seek(previousTime, false) correctly; only this path regressed.

Suggested shape: cycle progress past 0 to force the set render, then totalTime(Math.max(0, state.currentTime || 0), false) to honor the prior scrub. Same effect for the t=0 case (still kicks GSAP off the creation point) but doesn't drop user scrub state.

2. Array __timelines normalization is one-shot at init — most agent scripts will miss it (Pattern #2 decorative gate)

init.ts L79–91. The normalization runs once inside initSandboxRuntimeModular(). But the comment three blocks down (L2122–2125) says explicitly: "inline scripts registering child timelines in __timelines haven't executed yet (they run in the browser's next microtask). Defer a rebinding attempt to catch them." In other words, the populate-path runs after init, by design. An agent script that does window.__timelines = [tl] post-init lands in the populate phase the gate doesn't cover — the array form persists, and downstream timelines[compositionId] lookups (L452, L559, L2152) silently miss because the keys are "0", "1" instead of composition IDs.

Two ways out:

• Wrap window.__timelines in a setter (Object.defineProperty) that re-normalizes on every assignment.
• Or do the normalization opportunistically inside resolveRootTimelineFromDocument / the standalone-seek helpers — anywhere we read window.__timelines, normalize-on-read.

The init-time gate as written only helps the (presumably rare) case of window.__timelines = [tl] evaluated synchronously before initSandboxRuntimeModular() returns.

3. Nice-to-haves and confirmations

• data-start="0" default on root composition — interacts fine with #1662's ancestor walk. The root has no [data-start] ancestors (the walk breaks at rootComp), and the root will always have descendants so isTimedClipLeaf returns false (no display:none). Safe.
• immediateRender: true cross-PR with #1686 overlay validator — validator only inspects intent / overlap, not GSAP vars-bag keys, so no rejection. Generator and runtime are now in agreement on set() semantics — regime drift closed.
• Position-only set tween filtering / drag teardown translate: none clear — both look correctly scoped; the teardown's "do NOT clearProps:transform" comment is exactly the kind of band-aid-avoidance note the file needed.
• Ease-only keyframe update (gsapParser.ts L2227+) — preserves existing properties via isObjectProperty/propKeyName discovery. Reasonable; the only nit is the early-return short-circuit assumes ease is non-empty truthy, so an explicit ease: "" (clear-ease intent) would fall through to the existing buildKeyframeValueNode path and nuke the property bag. Probably not a real path, but worth a comment if it could be.
• No tests added for any of the new behaviors. The test plan is all manual. The array-normalization in particular would be a single-shot unit test (window.__timelines = [tl]; init(); expect(window.__timelines).toEqual({...})).

CI

At review time: Preflight + Detect changes + WIP passing; regression-shards, Perf:*, and Preview parity still in progress. Worth a glance before merging in case any GSAP-set-tween regression test trips on the new behavior.

Prior reviews

None yet (Via is first to land). Rames was tagged in parallel — recommend reconvening on findings #1 and #2 before merge.

Review by Via

R1 @ 7498be8 — Rames D Jusso review

Reviewing runtime resilience fixes. The set-tween / array-normalization / drag-teardown core is sound; two finds are load-bearing — a silent revert of a benchmarked perf PR, and a busted runtime debug surface.

🛑 Blockers

• packages/core/src/runtime/media.ts:289-308 — silent revert of #1651 (b651a9d). James's perf commit two days ago added a skipForInjectedVideo gate that skipped per-tick el.currentTime = relTime + el.load() drift recovery on <video> elements with sibling <img id="__render_frame_<id>__"> (frame-injection pipeline, render mode only). #1651 was validated N=3 baseline vs N=3 with-fix on synth-30-heavy: wall -1.8% (-2.2s), avgScreenshot -2.0%, avgBeforeCapture -7.0%, identical output MD5. This PR drops the gate entirely — git diff b651a9d2..7498be80 -- packages/core/src/runtime/media.ts shows the entire skipForInjectedVideo block removed without mention in the PR title, body, or commit message. Either restore the gate (it's orthogonal to the immediateRender fix) or document the rollback rationale + accept the per-render wall regression (~1.8% on video-heavy renders, ~2400 wasted seeks per 30×3s render). My read is unintentional rebase loss — please confirm.

• packages/core/src/runtime/diagnostics.ts:47-50 — swallow() now has if (w.__hfDebug || w.__HYPERFRAMES_DEBUG) { /* eslint-disable-next-line no-console -- intentional debug surface */ } with no body. The whole point of the function per the docstring (lines 14-28) is the opt-in __hfDebug console surface. This silently kills the documented runtime debug pathway in core (not studio — this is packages/core, out of #1691's scope but the change snuck into this PR). The orphaned doc-comment lines 17-19 are also now nonsense (" when window.__hfDebug === true …" with no preceding bullet about console.debug). Restore the console.debug call AND fix the docstring, or document the new contract (presumably "only the __hf.onSwallowed handler path is supported").

⚠️ Concerns

• packages/core/src/runtime/init.ts:79-91 (array timeline normalization) — uses document.querySelector("[data-composition-id]")?.getAttribute("data-composition-id") to pick the root composition ID. The runtime's authoritative resolver resolveRootCompositionElement() at line 247-263 of the same file checks data-root="true" first, falls back to topmost-non-nested, then first. For a single-comp page (Miguel's stated agent-mistake target), the two agree. For multi-comp pages with sub-compositions or an explicit data-root marker that isn't first in DOM order, the array-normalization picks the wrong key. The bridge to use is resolveRootCompositionElement itself — but it's declared later in the same function and depends on closures. Two options: (a) hoist the resolver function earlier, (b) replicate its priority logic inline (check data-root="true" first). As written, the array → object normalization works for the common case but lies about which composition owns the single timeline on multi-comp pages.

• packages/core/src/runtime/init.ts:96-99 (data-start defaulting) — same querySelector("[data-composition-id]") shape. Same multi-comp caveat. Lower-stakes because authoring without data-start on a non-root composition is already broken-shaped, but worth aligning with the resolver for consistency.

• packages/core/src/runtime/init.ts:1032-1038 — new if (boundDuration <= 0) branch calls progress(1, true); progress(0, false); pause(). For a forever-looping ambient timeline (tl.repeat(-1) with finite boundDuration ~ 0 from the safe-duration accessor), progress(1) forces it to end state then pause — which may visually snap mid-loop in unexpected ways at init. Hard to reason about without seeing what shape returns boundDuration <= 0 from getSafeTimelineDurationSeconds(_, 0). Worth a comment + a test for "infinite-loop timeline rebind doesn't visually snap to end-frame."

• packages/core/src/runtime/init.ts:1046-1053 — replacement of totalTime(seekTime, false) with the progress(0.0001, true); progress(0, false); pause() cycle. Hard to evaluate: does this respect state.currentTime? Previously totalTime(seekTime, false) honored a non-zero soft-reload restore time. The new shape always seeks to ~0 (progress(0)). Was the soft-reload state.currentTime > 0 restore intentionally dropped, or did the lateral on the fix lose it? Test for "soft-reload at t=2s shows frame@2s, not frame@0".

• packages/core/src/parsers/gsapParser.ts:1295 — tl.set always emits immediateRender: true. Inverse op (memory: feedback_inverse_operation_tolerance_asymmetry): the PARSE path (extras: { immediateRender }) at line 527 / 880 of the stress test already tolerates this. ✅ round-trip clean. But: position-only tl.set per the PR description is "hidden from timeline diamonds" — verify the diamond-filter check matches immediateRender: true rather than just method=="set" + position-only. Otherwise pre-PR saved compositions with set tweens lacking immediateRender still show diamonds while new ones don't — UI inconsistency.

• packages/core/src/parsers/gsapParser.ts:2227-2243 — ease-only keyframe update preserves existing properties for ObjectExpression values. ✅. But for non-ObjectExpression values (primitive — e.g. keyframes: { "50%": "0.5" } — uncommon but legal in GSAP's keyframe shorthand), it falls through to buildKeyframeValueNode(properties={}, ease) at line 2244 which wipes properties. Either short-circuit-return on non-ObjectExpression or accept the corner. Likely fine; flag as nit.

• Backward-compat (memory: status-machine in-flight rows gap): array-shaped timelines saved to disk before this PR worked because something else normalized them downstream? Or did they fail-silent and users blamed GSAP? Either way, post-PR they normalize at init. Migration concern is one-way: a comp authored against post-PR array-shaped runtime that gets opened in pre-PR runtime breaks. Minor — agents don't load Studio in old runtimes — but worth a one-liner in PR body.

• Test coverage — diff shows no test additions for: array normalization (window.__timelines = [tl] → keyed), data-start default, drag-translate-none clear, immediateRender emission, ease-only keyframe preservation. Each of these is a behavior addition that should have a unit test, especially the parser changes (gsapParser.stress.test.ts already has a similar shape at line 872-880). Filing as a test-debt concern rather than blocker because the runtime ones are hard-to-unit-test, but parser-level should pin.

🟡 Questions

• packages/studio/src/hooks/gsapRuntimeBridge.ts:235-251 refactor extracts hasNonHold to a variable + removes the explanatory comment. Pure refactor, no behavior change? Wanted to confirm the deleted comment lines 246-251 of the prior file ("STATIC case (single source of truth = GSAP timeline) …") were preserved elsewhere or intentionally dropped. The shape is non-obvious to readers; that comment was load-bearing for onboarding. Suggest re-adding it above the if (!hasNonHold) branch.

🟢 Looks good

• packages/studio/src/components/editor/manualOffsetDrag.ts:446-453 (#1673 drag teardown) — narrow + correctly defensive. Only clears translate: none literally, preserves committed transform. ✅
• packages/core/src/parsers/gsapWriterAcorn.ts:324-331 — updates.easeEach ?? updates.ease consolidation for keyframe-aware writers is clean. Comment loss is fine; the code reads.
• addAnimationWithKeyframesToScript(_, easeEach?) plumbing through both parsers — symmetric ✅.

— Rames D Jusso

Stamp routing: @rames Jusso once findings addressed (or punted with rationale).